Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EN-207 Add asset folder codenames #274

Merged

Conversation

matus12
Copy link
Contributor

@matus12 matus12 commented Jul 30, 2024

Motivation

Adding support for asset folder codenames, so assets can be inserted into specific folder, referenced by its codename.

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

Try getting / adding asset folders with codenames, adding asset to a folder referenced by codename.

@matus12 matus12 requested review from pokornyd and a team as code owners July 30, 2024 13:49
{
return itm;
}
else if (itm.Folders != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would lose this unnecessary else statement and use null coalescing to make it a bit more compact. also, wouldn't folder be more fitting variable name than itm in the foreach? seems slightly more readable, but that's just a nitpick

        foreach (var folder in folders)
        {
            if (folder.Id == folderId)
            {
                return folder;
            }

            var nestedFolder = folder.Folders?.GetFolderHierarchyById(folderId);
            if (nestedFolder != null) //This is required so you don't stop processing if the root contains many folders (let the above foreach loop continue)
            {
                return nestedFolder;
            }
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, looks good, fixed.

{
return folder;
}
else if (folder.Folders != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@matus12 matus12 force-pushed the EN-207-add-asset-folder-codenames branch from 337c067 to c15bccd Compare August 8, 2024 12:58
@pokornyd pokornyd merged commit 308705e into kontent-ai:master Aug 8, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants